ENH: Bump elastix version to 2023-07-19, inc. SetInitialTransform#234
Conversation
|
@tao558 If |
No, this PR adds |
|
Interesting... Do you mean ITKElastix should upgrade to a newer revision of ITKRemoteModuleBuildTestPackageAction? Looking at |
|
Yes.
Written on cellphone, excuse my brevity.
…On Wed, Jul 19, 2023, 14:18 Niels Dekker ***@***.***> wrote:
Maybe the same thing as fixed here
<InsightSoftwareConsortium/ITKRemoteModuleBuildTestPackageAction#64>
and here
<InsightSoftwareConsortium/ITKRemoteModuleBuildTestPackageAction#65>
?
Interesting... Do you mean ITKElastix should upgrade to a newer revision
of ITKRemoteModuleBuildTestPackageAction?
—
Reply to this email directly, view it on GitHub
<#234 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AANVQ6OUDF24BG4TNFAPOPTXRAQI5ANCNFSM6AAAAAA2QCU2HA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
I agree with @dzenanz , please first update to the latest commit in |
07ca69c to
f31041c
Compare
|
"cxx-build-workflow / build-test-cxx (windows-2022)" builds fine now that pull request #235 commit 7bdc103 is merged 👍 However, python-build-workflow-dev / build-linux-py (7, 2014-x64) still fails, saying: Any suggestion? |
|
Looks like python-build-workflow-dev / build-macos-py (11) is still using C++14, while elastix really needs C++17 now. Specifically, 'is_integral_v' is introduced with C++17, while this particular compiler says:
The compiler appears to be run as Any suggestion where this should be specified? Update: I see now, it also says:
|
@N-Dekker Thanks for finding this. As ITK requires a minimum of C++17 it is reasonable to require external modules to do the same. This seems like a bug in |
|
@N-Dekker thanks for working on the initial transform support 👏 @N-Dekker This should be fixed by the itk-5.4rc01 builds, where C++17 is enabled by the default via ITK. It could be tested by setting versions as in this PR: InsightSoftwareConsortium/ITKSplitComponents#72. |
|
Thanks to you both @tbirdso and @thewtex So I guess ITKElastix should upgrade both ITKRemoteModuleBuildTestPackageAction (to include a fix for InsightSoftwareConsortium/ITKRemoteModuleBuildTestPackageAction#66 ) and ITK (to itk-5.4rc01 at least), before upgrading to the latest (C++17 based) SuperElastix/elastix, right? |
Yes, I agree. Here is a potential roadmap for upgrading ITKElastix to the latest Elastix build with C++17:
EDIT: Updated roadmap with progress. |
|
@N-Dekker FYI, ITKElastix is now updated to target ITK v5.4rc1 with C++17 build tools. Please rebase this PR on |
Follow-up to InsightSoftwareConsortium#211 commit 1043f10 "ENH: Bump elastix version to 2023-05-15, use C++17" Including: SuperElastix/elastix#936 SuperElastix/elastix@115d8e1 ENH: Add `ElastixRegistrationMethod::SetInitialTransform SuperElastix/elastix#933 SuperElastix/elastix@d33a0d5 BUG: Fix reference count in ConvertItkTransformBaseToSingleItkTransform SuperElastix/elastix#924 SuperElastix/elastix@71a4a9a ENH: Add TransformParameterFileName property to TransformixFilter SuperElastix/elastix#922 SuperElastix/elastix@9d76b86 ENH: Restore ELASTIX_BUILD_EXECUTABLE CMake option SuperElastix/elastix#909 SuperElastix/elastix@9e64269 ENH: ElastixRegistrationMethod writes initial transform parameter files SuperElastix/elastix#903 SuperElastix/elastix@2170592 ENH: Remove the letter 's' from "InitialTransformParametersFileName" SuperElastix/elastix#898 SuperElastix/elastix@691c358 BUG: Add InitialTransformParameterObject maps to registration result
Aims to avoid compile errors from macosx-10.9-universal2-3.11/Xcode_13.2.1 MacOSX12.1.sdk/AppleClang 13.0.0.13000029 saying: > error: no template named 'is_integral_v' in namespace 'std'; did you mean 'is_integral'?
805f89f to
9327b3c
Compare
tbirdso
left a comment
There was a problem hiding this comment.
LGTM. CI is now passing with the update to v5.4rc1 CI workflows. 🟢
|
Thanks for your approval Tom! For the sake of time, I'll just merge now! |
Follow-up to #211 commit 1043f10 "ENH: Bump elastix version to 2023-05-15, use C++17"
Including:
SuperElastix/elastix#936
SuperElastix/elastix@115d8e1
ENH: Add `ElastixRegistrationMethod::SetInitialTransform
SuperElastix/elastix#933
SuperElastix/elastix@d33a0d5
BUG: Fix reference count in ConvertItkTransformBaseToSingleItkTransform
SuperElastix/elastix#924
SuperElastix/elastix@71a4a9a
ENH: Add TransformParameterFileName property to TransformixFilter
SuperElastix/elastix#922
SuperElastix/elastix@9d76b86
ENH: Restore ELASTIX_BUILD_EXECUTABLE CMake option
SuperElastix/elastix#909
SuperElastix/elastix@9e64269
ENH: ElastixRegistrationMethod writes initial transform parameter files
SuperElastix/elastix#903
SuperElastix/elastix@2170592
ENH: Remove the letter 's' from "InitialTransformParametersFileName"
SuperElastix/elastix#898
SuperElastix/elastix@691c358
BUG: Add InitialTransformParameterObject maps to registration result